Skip to content

feat: fetch events-config from config registry in AnalyticsController - #10448

Merged
gauthierpetetin merged 43 commits into
mainfrom
feat/analytics-controller-fetch-events-config
Oct 1, 2026
Merged

gauthierpetetin merged 43 commits into
mainfrom
feat/analytics-controller-fetch-events-config

Conversation

@gauthierpetetin

@gauthierpetetin gauthierpetetin commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Extends ConfigRegistryApiService with a fetchEventsConfig method that hits the /v1/config/events-config endpoint (with ETag caching, same pattern as fetchConfig)
  • Adds configs.eventsConfig and eventsConfigEtag fields to ConfigRegistryController state, populated in parallel with network configs on each polling cycle
  • Wires up the previously-stubbed AnalyticsController#fetchEventsConfig to read eventsConfig from ConfigRegistryController:getState on init, updating in-memory event-purpose classification when a newer version is available

Test plan

  • yarn workspace @metamask/config-registry-controller run test — 101 tests pass
  • yarn workspace @metamask/analytics-controller run test — 260 tests pass
  • yarn changelog:validate — passes

🤖 Generated with Claude Code


Note

Medium Risk
Changes how analytics events are classified for consent, with breaking messenger requirements for Wallet and AnalyticsController hosts.

Overview
Adds a remote events-config path through the config registry and connects AnalyticsController to it for event purpose classification (product vs marketing).

Config registry: ConfigRegistryApiService gains fetchEventsConfig for /v1/config/events-config (ETag/304 caching like networks). ConfigRegistryController stores configs.eventsConfig and eventsConfigEtag, fetching events config in parallel with network configs on each poll.

Analytics: The former #fetchEventsConfig stub now reads ConfigRegistryController:getState on init, validates and persists config when the version changes, and refreshes on ConfigRegistryController:stateChanged (subscription keyed on eventsConfig.version). Wallet initialization delegates the new API action.

Breaking: Integrators must wire ConfigRegistryApiService:fetchEventsConfig for ConfigRegistryController, and delegate ConfigRegistryController:getState plus stateChanged to AnalyticsController.

Reviewed by Cursor Bugbot for commit 55da180. Bugbot is set up for automated code reviews on this repo. Configure here.

- Extend ConfigRegistryApiService with a fetchEventsConfig method hitting /v1/config/events-config
- Store the result in ConfigRegistryController state (configs.eventsConfig + eventsConfigEtag)
- Wire up AnalyticsController#fetchEventsConfig to read from ConfigRegistryController:getState on init

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@gauthierpetetin
gauthierpetetin requested review from a team as code owners September 24, 2026 14:13
gauthierpetetin and others added 8 commits September 24, 2026 16:28
- Fix analytics-controller dependency from workspace:^ to ^4.0.0 (constraints)
- Add config-registry-controller tsconfig references to analytics-controller
- Fix Prettier formatting in config-registry-controller source files
- Update README content

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove readonly from #eventPurposes and #eventsConfigVersion private
  fields so #fetchEventsConfig can update them after construction
- Fix changelog PR links from #10401 to #10448 in both packages

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add explicit return type to buildConfigRegistryState in AnalyticsController.test.ts (ESLint)
- Remove dead ?? null branch in ConfigRegistryController#fetchEventsConfig (coverage)
- Add tests for modified:true without etag and non-Error thrown in #fetchEventsConfig (coverage 100%)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add eventsConfig and eventsConfigEtag to expected state assertions.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add ConfigRegistryApiService:fetchEventsConfig delegation in wallet
  instance so the feature is not silently dead in wallet hosts
- Update stale Phase 1 comment on eventsConfig state field
- Fix #fetchEventsConfig doc comment to say 'version differs' not 'newer'
- Add @metamask/config-registry-controller dependency to changelog

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Critical integration and public type compatibility issues, plus missing classification coverage, remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 High severity

Open (2)
What changed in this PR

Adds remote events configuration fetching and integrates it with analytics event-purpose classification.

Changes:

  • Adds ETag-cached events-config API support.
  • Stores and polls events configuration.
  • Applies remote purposes during analytics initialization.
  • Updates wallet wiring, tests, dependencies, and changelogs.
File Reviewed changes
yarn.lock Updates dependency metadata.
README.md Updates the dependency graph.
packages/​wallet/​src/​initialization/​instances/​config-registry-controller/​config-registry-controller.ts Delegates the new API action; handler registration requires updates.
packages/​wallet/​src/​initialization/​instances/​config-registry-controller/​config-registry-controller.test.ts Updates wallet controller test setup and expectations.
packages/​config-registry-controller/​src/​index.ts Exports new events-config types and actions.
packages/​config-registry-controller/​src/​ConfigRegistryController.ts Stores and polls events configuration; the required public state field may break existing consumers.
packages/​config-registry-controller/​src/​ConfigRegistryController.test.ts Tests events-config polling behavior.
packages/​config-registry-controller/​src/​config-registry-api-service/​types.ts Defines events-config schemas and result types.
packages/​config-registry-controller/​src/​config-registry-api-service/​config-registry-api-service.ts Fetches and caches the events-config endpoint.
packages/​config-registry-controller/​src/​config-registry-api-service/​config-registry-api-service.test.ts Tests endpoint, validation, ETag, and error behavior.
packages/​config-registry-controller/​src/​config-registry-api-service/​config-registry-api-service-method-action-types.ts Adds the new service action type.
packages/​config-registry-controller/​CHANGELOG.md Documents config registry support.
packages/​analytics-controller/​tsconfig.lint.json Adds the config registry project reference.
packages/​analytics-controller/​tsconfig.json Adds the config registry project reference.
packages/​analytics-controller/​tsconfig.build.json Adds the config registry build reference.
packages/​analytics-controller/​src/​AnalyticsController.ts Loads and applies remote event purposes; classification behavior needs direct test coverage.
packages/​analytics-controller/​src/​AnalyticsController.test.ts Tests analytics initialization integration.
packages/​analytics-controller/​package.json Adds the config registry dependency.
packages/​analytics-controller/​CHANGELOG.md Documents analytics integration.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/config-registry-controller/src/ConfigRegistryController.ts Outdated
- Make eventsConfig optional in ConfigRegistryControllerState to avoid
  a breaking change for consumers that construct state without this field
- Register ConfigRegistryApiService:fetchEventsConfig mock in wallet
  config-registry-controller test registerDependencies

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

A critical state compatibility issue and a moderate configuration refresh issue remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (2)

Comment thread packages/config-registry-controller/src/ConfigRegistryController.ts Outdated
Preserves non-breaking addition — existing consumers that construct
state without this field continue to type-check.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved critical messenger compatibility and shared-circuit-breaker issues remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 High severity

Open (3)
Resolved since last review (1)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Add tests verifying remote event purposes drive classification

packages/​analytics-controller/​src/​AnalyticsController.ts:1069

The added tests only assert that the remote config is persisted, not that this map is used for classification. A regression here could leave the controller treating a fetched marketing-only event as product-only while all new tests still pass; add an init/integration assertion that tracks an event with remote purposes and verifies the resulting consent/purpose behavior.

Low severity Document wallet events-config integration in the changelog

packages/​wallet/​src/​initialization/​instances/​config-registry-controller/​config-registry-controller.ts:34

This changes the default wallet initialization to require and invoke a new config-registry action, which adds a consumer-visible events-config request/state behavior, but packages/wallet/CHANGELOG.md has no Unreleased entry for it. Please document this wallet integration change alongside the package changelog entries.

Comment thread packages/analytics-controller/src/AnalyticsController.ts
Comment thread packages/config-registry-controller/src/ConfigRegistryController.ts
… changelog

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
gauthierpetetin and others added 2 commits September 30, 2026 10:32
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@NicolasMassart NicolasMassart left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One point I would resolve before approval is the open discussion around the controller dependency semantics.

gauthierpetetin and others added 5 commits September 30, 2026 10:51
…alyticsController

Use subscribe selector to filter state changes by eventsConfig version,
and rely on messenger types to guarantee ConfigRegistryController is
registered rather than swallowing errors silently.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@mcmire mcmire left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor thing, but otherwise looks good.

Comment thread packages/wallet/CHANGELOG.md
Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>
NicolasMassart
NicolasMassart previously approved these changes Sep 30, 2026
mikesposito
mikesposito previously approved these changes Sep 30, 2026

@mikesposito mikesposito left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config-registry-controller changes look good!

…rvice

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
mcmire
mcmire previously approved these changes Sep 30, 2026

@mcmire mcmire left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good now!

mikesposito
mikesposito previously approved these changes Sep 30, 2026
@gauthierpetetin
gauthierpetetin dismissed stale reviews from mikesposito and mcmire via 55da180 October 1, 2026 05:03
@gauthierpetetin
gauthierpetetin added this pull request to the merge queue Oct 1, 2026
Merged via the queue into main with commit 53f6dec Oct 1, 2026
354 of 361 checks passed
@gauthierpetetin
gauthierpetetin deleted the feat/analytics-controller-fetch-events-config branch October 1, 2026 09:05
cryptodev-2s pushed a commit to cryptodev-2s/core that referenced this pull request Oct 5, 2026
Release of (after this [PR](MetaMask#10448)
got merged):

- `@metamask/analytics-controller` (major): 3.2.0 → 4.0.0
- `@metamask/config-registry-controller` (major): 4.0.0 → 5.0.0
- `@metamask/wallet` (patch): 16.0.0 → 16.0.1

And release of their dependents:
- `@metamask/assets-controller` (patch): 18.0.0 → 18.0.1
- `@metamask/network-controller` (patch): 37.0.0 → 37.0.1
- `@metamask/network-enablement-controller` (patch): 7.0.1 → 7.0.2

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> The release propagates major analytics and config-registry changes
with breaking messenger wiring across network-controller and wallet;
integrators must update delegations even though this PR only changes
versions and changelogs.
> 
> **Overview**
> This is a **monorepo release** (`1312.0.0` → `1313.0.0`) that cuts new
package versions and threads updated dependency ranges through the
workspace and `yarn.lock`—no application source changes in this diff.
> 
> **Headline releases:** `@metamask/analytics-controller` **4.0.0** and
`@metamask/config-registry-controller` **5.0.0** (majors whose
changelogs carry **breaking messenger** requirements: analytics must
allow `ConfigRegistryController:getState` / `stateChanged`; config
registry must allow `ConfigRegistryApiService:fetchEventsConfig` and
caches `/v1/config/events-config` in state).
`@metamask/network-controller` **37.0.1** depends on those majors, so
upgrading network ripples the same constraints to most controllers.
> 
> **Patch releases:** `@metamask/wallet` **16.0.1**,
`@metamask/assets-controller` **18.0.1**, and
`@metamask/network-enablement-controller` **7.0.2** mainly record
dependency alignment on `network-controller` **37.0.1** and/or
`config-registry-controller` **5.0.0**. Dozens of packages bump
`@metamask/network-controller` to `^37.0.1`; consumers of
`@metamask/wallet-cli` also pick up `@metamask/wallet` **16.0.1** and
analytics/config-registry majors.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
6033cc1. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants